Avoid redundant looping in property metadata - #29805
Conversation
| @@ -858,29 +858,37 @@ private void Create( | |||
| var principalProperty = property; | |||
| for (var i = 0; i < 10000; i++) | |||
There was a problem hiding this comment.
Is 10,000 really the appropriate number here?
There was a problem hiding this comment.
It should be the length of the longest FK chain allowable in the model
|
Can you say a bit more about what is happening here? What are the situations where the 10,000 limit will be hit? Also, which tests cover these cases? |
|
The code is traversing FKs. The 10000 limit will be hit if there's an FK cycle in the model. These models are invalid and will fail validation, but the current code runs before validation, so it shouldn't fail before validation runs. Tests: |
|
The customer report didn't fail in validation. I get this was likely a bug, but it makes me nervous that we still have a very high limit such that some other bug could cause very slow model building without failing in validation. Should we not just fail early when we exhaust the loop, rather than assuming model validation will fail? |
Sure, but only in main |
|
Only in main seems reasonable. |
3f93fcc to
f5bc9b0
Compare
f5bc9b0 to
bc1fdcc
Compare
Fixes #29642